Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suppression de redux-form #671

Merged
merged 7 commits into from
Sep 23, 2019
Merged

Suppression de redux-form #671

merged 7 commits into from
Sep 23, 2019

Conversation

mquandalle
Copy link
Contributor

@mquandalle mquandalle commented Sep 12, 2019

Fixes #632

La suppression de cette dépendance simplifie grandement le code, qui est plus concis (alors que l'on supprime une dépendance !) et plus clair (moins d'indirections). La PR est a parité de fonctionnalités. Cette refacto doit permettre d'enchaîner sur les modifications mentionnées dans #632.

@mquandalle mquandalle force-pushed the remove-redux-form branch 3 times, most recently from fd21cfe to ee92b3b Compare September 13, 2019 10:48
@mquandalle mquandalle marked this pull request as ready for review September 13, 2019 10:48
@mquandalle mquandalle force-pushed the remove-redux-form branch 5 times, most recently from cfd4fd0 to e9eddce Compare September 13, 2019 13:06
Copy link
Contributor

@johangirod johangirod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 👏 👏 Une refacto vraiment top ! L'usage des hooks simplifie énormément la lecture et la logique. Et avoir la situation dans le state permet enfin de déplacer une la logique métier des composants vers les reducer 👍

source/components/PeriodSwitch.js Outdated Show resolved Hide resolved
source/components/TargetSelection.js Outdated Show resolved Hide resolved
source/components/conversation/Input.js Show resolved Hide resolved
source/components/conversation/Input.js Outdated Show resolved Hide resolved
En vue de la suppression de Redux-form, ce commit crée deux nouvelles
actions : UPDATE_SITUATION et UPDATE_PERIOD qui permettent de gérer le
state de la situation, en retrouvant le même résulat qu'avec l'ancienne
implémentation au niveau du `formattedSituationSelector`
@mquandalle mquandalle force-pushed the remove-redux-form branch 2 times, most recently from 98563f2 to 17810ac Compare September 17, 2019 10:49
Supprime aussi redux-batched-action. Le code résultant est plus concis
(alors que l'on supprime une dépendance !), et plus clair car il y a moins
d'indirections pour se conformer aux API de redux-form.
Comme recommandé dans la documentation des hooks React, ajout des deux
linters suivants : react-hooks/rules-of-hooks et react-hooks/exhaustive-deps

Mise à jour des composants, en particulier les useEffect pour y spécifier
toutes les dépendances.
Nous alternions avant entre un <span /> et un <input /> selon le contexte

Fixes #558
@mquandalle
Copy link
Contributor Author

J'ai ajouté un commit qui laisse toujours l'input affiché pour les objectifs du simulateurs — plutôt que d'alterner entre input et span comme actuellement.

@mquandalle
Copy link
Contributor Author

Remarque : le dernier commit que j'ai poussé hier soit (pour mettre rules dans le state) est buggé. Le déploiement est toujours dispo sur le commit précédent.

@johangirod
Copy link
Contributor

Le deploy du dernier commit qui marche :
https://5d80bbbfe4532f0008308390--syso.netlify.com/s%C3%A9curit%C3%A9-sociale/ind%C3%A9pendant

Je te laisse regarder les bugs avant de reviewer :)

@johangirod
Copy link
Contributor

Petit bug sur la version anglaise :

chrome-capture (1)

Déplace la logique de changement de période d'un component vers un reducer
@mquandalle
Copy link
Contributor Author

mquandalle commented Sep 18, 2019

J'ai réparé le commit qui déplace les règles dans le state redux.

@johangirod J'ai pas trop compris le bug : c'est le positionnement du curseur qui saute à la fin (ce que je n'arrive pas à reproduire), ou le fait qu'il y ait des animations de changement de prix en trop ?

Suppression de l'AnimatedValue pour l'objectif courant
Simplification du code de TargetSelection
Corrections CSS
@mquandalle mquandalle force-pushed the remove-redux-form branch 2 times, most recently from f224373 to da077aa Compare September 23, 2019 09:39
@mquandalle
Copy link
Contributor Author

Dans la foulée je voudrais centraliser les différentes fonctions de formatage et normalisation des valeurs qui sont éparpillées dans le code, mais je propose de merger cette PR indépendamment.

@mquandalle mquandalle merged commit f3e79f4 into master Sep 23, 2019
@mquandalle mquandalle deleted the remove-redux-form branch September 25, 2019 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suppression de Redux-form
3 participants